-
Notifications
You must be signed in to change notification settings - Fork 9
Create Dockerfile #133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Create Dockerfile #133
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed b44296a. In addition to my comments, the zizmor failures need to be fixed.
echo "zallet_version=$(echo ${{ github.ref_name }} | sed 's/v//g')" >> $GITHUB_OUTPUT | ||
|
||
build_push: | ||
uses: zcash/.github/.github/workflows/build-and-push-docker-hub.yaml@main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this doing? It looks recursive to me which makes no sense. Is this actually pulling from the zcash/zcash
repo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is the template we're using in Zcash and across several of our other repositories. - Just realized I missed updating a path — just fixed it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments for better UX and reproducibility.
Dockerfile
Outdated
|
||
COPY . . | ||
|
||
RUN cargo build --release && strip target/release/zallet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using strip
makes debugging harder. If the stripped zallet
binary crashes inside the distroless container, the stack traces will be much less informative because the symbol information is not available. This is a good example: https://stackoverflow.com/a/73628573
Adding the --locked
flag is recommended for reproducibility, as it forces Cargo to use only the versions of dependencies specified in the Cargo.lock
file. This is also aligned with the use of cargo vet --locked
in the /.github/workflows/audits.yml
workflow
Optional: Adding --package zallet --bin zallet
for visibility and explicitness. Also, if in the future another package is added to the [workspace.members]
cargo build without --package
might try to build all members or a different default. Similar with --bin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@str4d any thoughts on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional: This can be a reusable workflow. For example:
This commit introduces some optimizations to the Docker build process: 1. **Build Caching**: * Leverages Docker BuildKit cache mounts for `cargo` dependencies (`/app/.cargo`) and compiled artifacts (`/app/target/`). This speeds up subsequent builds by reusing downloaded and compiled dependencies. * Uses bind mounts for `zallet` source, `Cargo.toml`, and `Cargo.lock` to ensure the build step has access to the necessary files during the cached `RUN` instruction. * The `--link` flag is now used with `COPY` for potentially faster copies when supported. 2. **Optimized Build Command**: * The `cargo build` command is now more explicit, specifying `--locked --package zallet --bin zallet`. * The `strip` command has been removed to allow runtime debugging information. 3. **Explicit Naming and Structure**: * The runtime stage is now explicitly named `AS runtime`. * The `ENTRYPOINT` now uses the simple binary name `zallet`, relying on it being in the `PATH`. 4. **`.dockerignore` File**: * A `.dockerignore` file has been added to control the build context sent to the Docker daemon. * It follows an "exclude all, then include specific" pattern (`*` followed by `!pattern`) to ensure only necessary files (`.cargo`, `*.toml`, `*.lock`, and the `zallet` source directory) are included in the build context. This reduces the context size and can prevent unnecessary cache invalidations.
I opened #150 targeting this PR, with minimal design changes, but adding some of the optimizations mentioned in the comments |
Explicit version of RUST
feat(docker): Optimize build with cache mounts and `.dockerignore`
This PR adds a multi-stage Docker build for the
zallet
binary, resulting in a minimal and secure container image.Build Stage (
builder
)rust:1-slim
(amd64).clang
,libclang-dev
,pkg-config
,git
).--release
mode.strip
to reduce binary size.Runtime Stage (
distroless
)gcr.io/distroless/cc
for a minimal and secure runtime environment.zallet
binary.Additional Changes
This setup ensures small image size, strong security practices, and a clear separation between build and runtime environments.